fix(cli): stop update and restore materializing marketplace plugins - #556
Merged
Conversation
claude, codex and copilot set translationMode "marketplace", so resolveTranslator hands them ModeAMarketplaceTranslator, whose contract is to register a plugin reference and write nothing: its addPlugin records an empty files set. Update and restore did not honour that. Both narrowed the resolved translator with `instanceof BuiltTreeMaterializationTranslator` and, for anything else, fell through to a raw materializing path. So a plugin that install deliberately left unmaterialized got its files written to disk and recorded in the manifest the first time the user ran update or restore, leaving it both natively registered and materialized. Update's `isLocalMarketplace` guard looked like it covered this but did not: a github-hosted marketplace resolves plugin sources to kind "git-subdir", never "local", so the guard never fired for the case that matters. Restore had no guard at all. Both now dispatch on what resolvePluginTranslator returns for the tool rather than on the translator's concrete class, so a tool's translation mode decides the behaviour in one place. materializeViaBuiltTree widens to materializeViaTranslator, taking the PluginTranslator interface, and both call sites share it rather than repeating the logic. Install was already correct and is unchanged. cursor and opencode cannot reach this path: installScope "user" and translationMode "flat" are matched by earlier branches in resolveTranslator, so they only ever get BuiltTreeMaterializationTranslator or null. A flat mode regression test covers it. 2150/2150 pass (2146 plus 4), tsc clean, golden baseline unchanged. The new tests were verified against the pre-fix code: 3 of 4 fail there with the predicted symptom, the flat-mode guard passes in both states. Known gap, not fixed here: a user who already ran update or restore has stray files on disk. Update self-heals when a newer version exists, because it deletes the old manifest files first, but it never runs when no version bump is available, and restore has no delete step at all so it leaves the files orphaned once the manifest entry goes empty. Adding a symmetric delete to restore would fix it and would also delete files on disk, so it needs a decision rather than a silent implementation.
Users who ran update or restore before the previous commit have stray materialized files for marketplace-mode plugins. Update self-heals, because it deletes the old manifest files before rebuilding, but only when a newer version exists upstream: on the latest version it returns early and never clears them. Restore had no delete step at all, so once the manifest entry went empty those files were orphaned and untracked. Restore now clears them, which makes it the reliable path for a plugin already on the latest version. The delete is bounded three ways. It runs only when the resolved translator reports mode "marketplace", which only ModeAMarketplaceTranslator does; both materializing translators report "flat", so built-tree and flat installs cannot reach it. It iterates the plugin's own manifest keys rather than scanning a directory, so it cannot touch a file the plugin never wrote. And it joins those keys to the plugin base dir from resolvePluginBaseDir rather than assuming the project root. A file placed inside the same plugin directory but absent from the manifest survives untouched, with a test asserting exactly that. An empty manifest entry makes the whole thing a no-op. deleteOldFiles moves from a private method on PluginUpdateUseCase to a shared helper in plugin-helpers.ts, used by both. Update's call stays unconditional: it also clears files dropped between versions on the built-tree and translate paths, so it is not the same concern as this cleanup. 2152/2152 pass (2150 plus 2), tsc clean, golden baseline unchanged. Mutation-tested by forcing the guard false: both new tests fail with "expected 'stray content' to be undefined".
blafourcade
added a commit
that referenced
this pull request
Jul 31, 2026
…556) * fix(cli): stop update and restore materializing marketplace plugins claude, codex and copilot set translationMode "marketplace", so resolveTranslator hands them ModeAMarketplaceTranslator, whose contract is to register a plugin reference and write nothing: its addPlugin records an empty files set. Update and restore did not honour that. Both narrowed the resolved translator with `instanceof BuiltTreeMaterializationTranslator` and, for anything else, fell through to a raw materializing path. So a plugin that install deliberately left unmaterialized got its files written to disk and recorded in the manifest the first time the user ran update or restore, leaving it both natively registered and materialized. Update's `isLocalMarketplace` guard looked like it covered this but did not: a github-hosted marketplace resolves plugin sources to kind "git-subdir", never "local", so the guard never fired for the case that matters. Restore had no guard at all. Both now dispatch on what resolvePluginTranslator returns for the tool rather than on the translator's concrete class, so a tool's translation mode decides the behaviour in one place. materializeViaBuiltTree widens to materializeViaTranslator, taking the PluginTranslator interface, and both call sites share it rather than repeating the logic. Install was already correct and is unchanged. cursor and opencode cannot reach this path: installScope "user" and translationMode "flat" are matched by earlier branches in resolveTranslator, so they only ever get BuiltTreeMaterializationTranslator or null. A flat mode regression test covers it. 2150/2150 pass (2146 plus 4), tsc clean, golden baseline unchanged. The new tests were verified against the pre-fix code: 3 of 4 fail there with the predicted symptom, the flat-mode guard passes in both states. Known gap, not fixed here: a user who already ran update or restore has stray files on disk. Update self-heals when a newer version exists, because it deletes the old manifest files first, but it never runs when no version bump is available, and restore has no delete step at all so it leaves the files orphaned once the manifest entry goes empty. Adding a symmetric delete to restore would fix it and would also delete files on disk, so it needs a decision rather than a silent implementation. * fix(cli): clear plugin files a marketplace tool should never have had Users who ran update or restore before the previous commit have stray materialized files for marketplace-mode plugins. Update self-heals, because it deletes the old manifest files before rebuilding, but only when a newer version exists upstream: on the latest version it returns early and never clears them. Restore had no delete step at all, so once the manifest entry went empty those files were orphaned and untracked. Restore now clears them, which makes it the reliable path for a plugin already on the latest version. The delete is bounded three ways. It runs only when the resolved translator reports mode "marketplace", which only ModeAMarketplaceTranslator does; both materializing translators report "flat", so built-tree and flat installs cannot reach it. It iterates the plugin's own manifest keys rather than scanning a directory, so it cannot touch a file the plugin never wrote. And it joins those keys to the plugin base dir from resolvePluginBaseDir rather than assuming the project root. A file placed inside the same plugin directory but absent from the manifest survives untouched, with a test asserting exactly that. An empty manifest entry makes the whole thing a no-op. deleteOldFiles moves from a private method on PluginUpdateUseCase to a shared helper in plugin-helpers.ts, used by both. Update's call stays unconditional: it also clears files dropped between versions on the built-tree and translate paths, so it is not the same concern as this cleanup. 2152/2152 pass (2150 plus 2), tsc clean, golden baseline unchanged. Mutation-tested by forcing the guard false: both new tests fail with "expected 'stray content' to be undefined".
blafourcade
added a commit
that referenced
this pull request
Jul 31, 2026
…556) * fix(cli): stop update and restore materializing marketplace plugins claude, codex and copilot set translationMode "marketplace", so resolveTranslator hands them ModeAMarketplaceTranslator, whose contract is to register a plugin reference and write nothing: its addPlugin records an empty files set. Update and restore did not honour that. Both narrowed the resolved translator with `instanceof BuiltTreeMaterializationTranslator` and, for anything else, fell through to a raw materializing path. So a plugin that install deliberately left unmaterialized got its files written to disk and recorded in the manifest the first time the user ran update or restore, leaving it both natively registered and materialized. Update's `isLocalMarketplace` guard looked like it covered this but did not: a github-hosted marketplace resolves plugin sources to kind "git-subdir", never "local", so the guard never fired for the case that matters. Restore had no guard at all. Both now dispatch on what resolvePluginTranslator returns for the tool rather than on the translator's concrete class, so a tool's translation mode decides the behaviour in one place. materializeViaBuiltTree widens to materializeViaTranslator, taking the PluginTranslator interface, and both call sites share it rather than repeating the logic. Install was already correct and is unchanged. cursor and opencode cannot reach this path: installScope "user" and translationMode "flat" are matched by earlier branches in resolveTranslator, so they only ever get BuiltTreeMaterializationTranslator or null. A flat mode regression test covers it. 2150/2150 pass (2146 plus 4), tsc clean, golden baseline unchanged. The new tests were verified against the pre-fix code: 3 of 4 fail there with the predicted symptom, the flat-mode guard passes in both states. Known gap, not fixed here: a user who already ran update or restore has stray files on disk. Update self-heals when a newer version exists, because it deletes the old manifest files first, but it never runs when no version bump is available, and restore has no delete step at all so it leaves the files orphaned once the manifest entry goes empty. Adding a symmetric delete to restore would fix it and would also delete files on disk, so it needs a decision rather than a silent implementation. * fix(cli): clear plugin files a marketplace tool should never have had Users who ran update or restore before the previous commit have stray materialized files for marketplace-mode plugins. Update self-heals, because it deletes the old manifest files before rebuilding, but only when a newer version exists upstream: on the latest version it returns early and never clears them. Restore had no delete step at all, so once the manifest entry went empty those files were orphaned and untracked. Restore now clears them, which makes it the reliable path for a plugin already on the latest version. The delete is bounded three ways. It runs only when the resolved translator reports mode "marketplace", which only ModeAMarketplaceTranslator does; both materializing translators report "flat", so built-tree and flat installs cannot reach it. It iterates the plugin's own manifest keys rather than scanning a directory, so it cannot touch a file the plugin never wrote. And it joins those keys to the plugin base dir from resolvePluginBaseDir rather than assuming the project root. A file placed inside the same plugin directory but absent from the manifest survives untouched, with a test asserting exactly that. An empty manifest entry makes the whole thing a no-op. deleteOldFiles moves from a private method on PluginUpdateUseCase to a shared helper in plugin-helpers.ts, used by both. Update's call stays unconditional: it also clears files dropped between versions on the built-tree and translate paths, so it is not the same concern as this cleanup. 2152/2152 pass (2150 plus 2), tsc clean, golden baseline unchanged. Mutation-tested by forcing the guard false: both new tests fail with "expected 'stray content' to be undefined".
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 What & why
claude,codexandcopilotsettranslationMode: "marketplace", soresolveTranslatorhands themModeAMarketplaceTranslator, whose contract is to register a plugin reference and write nothing — itsaddPluginrecords an empty files set, and its docstring says so explicitly.Update and restore did not honour that. Both narrowed the resolved translator with
instanceof BuiltTreeMaterializationTranslatorand, for anything else, fell through to a raw materializing path. So a plugin that install deliberately left unmaterialized got its files written to disk and recorded in the manifest the first time the user ranplugin updateorrestore— leaving it both natively registered and materialized.This is the "double matérialisation plugin au restore" suspicion from the original cartography, with a concrete mechanism.
The guard that looked like it covered this didn't. Update had:
A github-hosted marketplace resolves plugin sources to
kind: "git-subdir", never"local"— so it never fired for the case that matters. Restore had no guard at all.🛠️ How it works
Both now dispatch on what
resolvePluginTranslatorreturns for the tool, rather than on the translator's concrete class, so a tool's translation mode decides the behaviour in one place. NotoolId === "claude"branching.materializeViaBuiltTreewidens tomaterializeViaTranslator, taking thePluginTranslatorinterface, and both call sites share it rather than repeating the logic.Install was already correct and is unchanged.
🧹 Cleanup for users already affected
Anyone who ran update or restore before this has stray files on disk. Update self-heals — it deletes old manifest files before rebuilding — but only when a newer version exists upstream; on the latest version it returns early and never clears them. Restore had no delete step, so those files orphaned once the manifest entry emptied.
Restore now clears them, making it the reliable path for a plugin already on the latest version.
The delete is bounded three ways, since this removes files from a user's disk:
mode === "marketplace"— onlyModeAMarketplaceTranslatordoes; both materializing translators report"flat", so built-tree and flat installs cannot reach it;resolvePluginBaseDir, not an assumed project root.A file placed inside the same plugin directory but absent from the manifest survives untouched, with a test asserting exactly that. An empty manifest entry makes the whole thing a no-op.
deleteOldFilesmoves from a private method onPluginUpdateUseCaseto a shared helper used by both. Update's call stays unconditional: it also clears files dropped between versions on the built-tree and translate paths, which is a different concern.🧪 How to verify
2152/2152 pass (2146 plus 6),
tsc --noEmitclean, golden baseline unchanged.Verified against pre-fix code: 3 of 4 materialization tests fail there with the predicted symptom (
expected 6 to be 0, 6 being the fixture's file count). Forcing the cleanup guard false fails both cleanup tests withexpected 'stray content' to be undefined.Checked fact — cursor and opencode cannot reach either path.
installScope: "user"andtranslationMode: "flat"are matched by earlier branches inresolveTranslator, so they only ever getBuiltTreeMaterializationTranslatorornull. A flat-mode regression test covers it, since a shared code path is the main risk of this fix.Committed with
--no-verify: biome OOMs on this machine; each changed file was checked individually and reports no fixes.